Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/opencode/src/altimate/native/connections/data-diff.ts">

<violation number="1" location="packages/opencode/src/altimate/native/connections/data-diff.ts:266">
P2: Missing single-quote escaping on `partitionValue` in the date-mode branches. The categorical mode (6 lines above) escapes with `.replace(/'/g, "''")`, but none of the date-mode `switch` cases do. Apply the same escaping for consistency and defense in depth.</violation>

<violation number="2" location="packages/opencode/src/altimate/native/connections/data-diff.ts:489">
P2: `partitionColumn` is not identifier-quoted in `buildPartitionDiscoverySQL`, but it is quoted in `buildPartitionWhereClause`. If the column name is a reserved word (e.g. `date`, `order`), the discovery query will produce a syntax error. Quote the column consistently between both functions.</violation>
</file>

<file name="packages/opencode/src/altimate/native/connections/registry.ts">

<violation number="1" location="packages/opencode/src/altimate/native/connections/registry.ts:125">
P2: `fabric` is missing from the `PASSWORD_DRIVERS` set. Since it maps to the same sqlserver driver as `sqlserver`/`mssql` (both present in the set), `fabric` should also be included to get the same friendly error when `password` is not a string.</violation>
</file>

<file name="packages/opencode/src/altimate/tools/data-diff.ts">

<violation number="1" location="packages/opencode/src/altimate/tools/data-diff.ts:206">
P2: When `d.values` is nullish, `d.values?.join(" | ")` evaluates to `undefined`, which the template literal coerces to the string `"undefined"`. The output would read e.g. `[source only] undefined`. Use a fallback to produce a sensible message.</violation>
</file>

<file name="packages/drivers/src/sqlserver.ts">

<violation number="1" location="packages/drivers/src/sqlserver.ts:148">
P1: `flattenRow` flattens all array values, but only the empty-string key (`""`) holds mssql's merged unnamed columns. A legitimate array column value (e.g. from JSON aggregation) will be incorrectly spread, corrupting the row data and misaligning columns. Restrict flattening to the `""` key only.</violation>
</file>

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/drivers/src/sqlserver.ts">

<violation number="1" location="packages/drivers/src/sqlserver.ts:90">
P2: Avoid `execSync` in the async connection path; it can block the event loop for up to the CLI timeout.</violation>
</file>

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/opencode/src/altimate/native/connections/data-diff.ts">

<violation number="1" location="packages/opencode/src/altimate/native/connections/data-diff.ts:819">
P2: `crossWarehouse` is computed from unresolved params, so an omitted `source_warehouse` can be misclassified as cross-warehouse when both sides actually run on the same default warehouse.</violation>
</file>

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.opencode/skills/data-parity/SKILL.md:
- Around line 453-458: The fenced code block showing the pseudo-YAML config (the
block containing type: "fabric", host:
"<workspace-id>-<item-id>.datawarehouse.fabric.microsoft.com", database:
"<warehouse-name>", authentication: "azure-active-directory-default") is missing
a language tag; change the opening fence from ``` to ```yaml (or ```text) so
markdownlint MD040 is satisfied and the block is treated as YAML.

In `@packages/drivers/package.json`:
- Around line 20-33: Remove the redundant optional peer dependency entry for
"@azure/identity" from package.json: delete the "@azure/identity" key inside the
"peerDependencies" object and its corresponding entry in "peerDependenciesMeta"
since "mssql"/"tedious" already bundles it; update the package.json so only the
other dependencies (e.g., "mssql", "oracledb", "duckdb", "mongodb",
"@clickhouse/client") remain and no "@azure/identity" references exist under
"peerDependencies" or "peerDependenciesMeta".

In `@packages/drivers/src/sqlserver.ts`:
- Around line 53-54: The code calls rawAuth.toLowerCase() without ensuring
rawAuth is a string, causing a TypeError for non-string authentication values;
update the computation of authType in the sqlserver driver to first check typeof
rawAuth === "string" before using toLowerCase and looking up AUTH_SHORTHANDS,
e.g. only apply AUTH_SHORTHANDS[rawAuth.toLowerCase()] when rawAuth is a string,
otherwise pass rawAuth through (or leave undefined) so non-string auth blocks
(config.authentication) don't blow up at runtime; adjust the logic around
rawAuth, authType, and AUTH_SHORTHANDS to be type-safe and explicit.
- Around line 56-158: The code path that handles authType starting with
"azure-active-directory" (see authType variable and mssqlConfig.authentication
assignments) does not handle unknown subtypes and can leave mssqlConfig without
authentication; add a terminal else branch after the existing
azure-active-directory-* branches that throws a clear Error (e.g. mentioning the
unsupported authType) so unknown or misspelled Azure AD subtypes fail fast;
ensure the thrown error references authType so callers get an explicit message
instead of a later opaque tedious error.
- Around line 76-87: The empty catch around the `@azure/identity` import and
credential.getToken is hiding real authentication errors; change the catch to
capture the thrown error (e.g., catch (err) { azureIdentityErr = err }) and
preserve that error in a local variable, then either append its message/details
to the final thrown error or log it at debug level when token remains undefined;
update the block that currently throws the generic "install `@azure/identity` or
run az login" message to include the captured azureIdentityErr (or its
message/stack) so real AAD/authentication failures are visible while still
tolerating missing-module/browser-bundle cases.
- Around line 208-217: The current columns logic replaces known named headers
when sampleFlat is longer (mixed named + unnamed aggregates); change the
construction in the columns branch to preserve the existing named prefix
(namedKeys) and only synthesize column names for the extra flattened entries
(e.g., generate col_i for indices >= namedKeys.length up to
sampleFlat.length-1), falling back to result.recordset.columns only when both
namedKeys and sampleFlat are empty; update the conditional around
sampleFlat/namedKeys where columns is assigned (the expression that uses
sampleFlat, namedKeys, recordset, and result.recordset.columns) so known headers
are retained and only the tail is renamed.
- Around line 162-167: Remove the fallback to the global shared pool to avoid
reintroducing cross-database isolation; in the block that currently checks
MssqlConnectionPool, ensure that if MssqlConnectionPool is falsy you throw an
explicit error instead of calling mssql.connect, i.e., keep the branch that
constructs and connects a new MssqlConnectionPool(mssqlConfig) and replace the
else branch with a throw that references MssqlConnectionPool and mssql.connect
in the message so reviewers can quickly locate and understand the change.

In `@packages/drivers/test/sqlserver-unit.test.ts`:
- Around line 154-159: The test "empty result returns correctly" uses a fake
mssql response shape by setting mockQueryResult.recordset_columns, but the
driver reads result.recordset?.columns; update the test to match real mssql
shape by removing recordset_columns and instead set mockQueryResult.recordset =
[] and, if you need to include column metadata, set
mockQueryResult.recordset.columns = {} (or leave absent for empty results) so
the driver code that checks result.recordset?.columns and the code paths in
connector.execute behave against the real response shape.

---

Nitpick comments:
In `@packages/drivers/src/sqlserver.ts`:
- Around line 89-100: The current token fetch inside connect() uses sync child
process invocation (execSync) which blocks the event loop; replace it with an
asynchronous call (e.g., use child_process.execFile or promisified exec via
util.promisify) so the Azure CLI call runs without blocking. Locate the block
that imports execSync and sets token (the token variable and the try/catch that
calls "az account get-access-token ...") and change it to call the async API
(await the execFile/promisified exec) with the same args and timeout, handle
stdout.trim() to set token, and preserve the existing catch behavior for missing
CLI or auth failures.

In `@packages/drivers/test/sqlserver-unit.test.ts`:
- Around line 483-489: Add a new test variant in
packages/drivers/test/sqlserver-unit.test.ts that uses a recordset like { name:
"alice", "": [42, 100] } (i.e., multiple unnamed values) and assert both
result.rows and result.columns so the header preserves the named column "name"
and includes generated column names for the unnamed values (e.g.,
["name","col_1","col_2"]); this will surface the bug referenced around
sqlserver.ts (logic near lines handling column mapping) and prevent regressions
in connector.execute's column assembly.

In `@packages/opencode/src/altimate/native/connections/data-diff.ts`:
- Around line 815-820: Summary: crossWarehouse currently does a raw textual
compare (sourceWarehouse !== targetWarehouse) which can misclassify undefined vs
an explicit default warehouse name as cross-warehouse. Fix: canonicalize both
sides before comparing by resolving undefined/empty values to the
effective/default warehouse name used elsewhere (use the same resolver used for
connection/effective settings) instead of comparing raw params.source_warehouse
and params.target_warehouse; update the variables referenced (sourceWarehouse,
targetWarehouse, crossWarehouse) so crossWarehouse = resolvedSourceWarehouse !==
resolvedTargetWarehouse. Ensure you call the existing resolution helper (or add
one) rather than inventing a new default string.

In `@packages/opencode/test/altimate/data-diff-dialect.test.ts`:
- Around line 51-54: Add assertions to the "handles uppercase input" test to
cover other inputs that share the normalization path so case-insensitivity can't
regress: update the test block in
packages/opencode/test/altimate/data-diff-dialect.test.ts that calls
warehouseTypeToDialect to also assert uppercase variants such as "MSSQL" ->
"tsql", "FABRIC" -> "fabric" and "MARIADB" -> "mariadb" (or whichever expected
dialects map in warehouseTypeToDialect) so all branches that are lowercased are
exercised.